Applied comments from Livecoms Reviewer #64
Conversation
|
@akohlmey Something that came to my mind while applying the reviewer's comments, would it be difficult to have the units appear on the axis of the LAMMPS-GUI Charts when printing thermo keywords of LAMMPS, such as "Density":
|
|
@akohlmey @jrgissing What do you think of this reviewer's comment?
Would there be a better suffix for such files? |
|
@akohlmey @jrgissing Another interesting comment concerning the use in tutorial 6 of :
Apart from it being a bad combination, a reviewer suggest using hybrid instead of hybrid/overlay. I am actually thinking that it would be best to completely remove the "hybrid/overlay vashishta" from this part, and then only have one single potential lj/cut/tip4p/long, leaving the silica rigid as is not uncommon for GCMC simulations. It would be cleaner, but then there would not be a single use of hybrid pair style in the entire tutorials. What do you think is best? |
Yes, because we have no idea what those units would be. The text for thermo columns can be changed to anything with What could be useful here is a) display the LAMMPS units settings and b) if the thermo output is normalized (like for units lj by default) since that can be changed with |
Following the convensions used in the LAMMPS potentials folder, the name for this file would become: |
The comment is correct. Pair style hybrid/overlay is not needed. Unfortunately, this is a convention that was started by Andrew Jewett's moltemplate and is propagating. Pair style hybrid/overlay gives you the most flexibility when combining multiple force fields (but also the largest margin for error due to lack of mixing and potential double counting of interactions). Yes, vashishta can only be used reliably for bulk systems. I think making the silica immobile (don't use the term rigid here, rigid is still mobile but as a whole) is the best solution without having to have major edits to the paper. I think the issue of hybrid potentials is worth a more general treatment, like in a Howto document in LAMMPS. That would allow to discuss examples rather than explain features like in the parts documenting the commands. That would allow, for example, to use some drastic words showing how bad using hybrid with EAM is. |
Noted, will do. |
|
@simongravelle We should require LAMMPS-GUI version 1.7 which is included in LAMMPS version 29Aug2024-update4 (not yet released) and LAMMPS version 30Jul2025 (not yet released and release date subject to change if we don't get all pending pull requests not flagged for after the stable release merged) |
|
Could you take care of the last remaining comment by Reviewer C?
|
|
There is an inconsistency between the gcmc.lmp file of tutorial 6 and what is shown in the pdf. I believe the .pdf is wrong. I will take the liberty to edit on github accordingly. 
|
Oh, yes, we decided to remove pair_style hybrid/overlay completely |
|
ah okay. So I will undo the changes I've just done. The .lmp file will then need to be modified |
Yes. We removed hybrid because it was extremely questionable, and one reviewer had a comment on that. Its creaner to simply not use hybrid, and will prevent future users to reproduce such bad habit. |
|
No problem :) |
| \label{eq:G} | ||
| \end{equation} | ||
| where $\Delta G$ is the free energy difference, $R$ is the gas constant, $T$ | ||
| is the temperature, $p$ is the pressure, and $p_0$ is a reference pressure. | ||
| is the temperature, % $p$ is the pressure, and $p_0$ is a reference pressure. | ||
| {\color{blue}$\rho$ is the density, and $\rho_0$ is a reference density.} | ||
| As an illustration, let us apply this method to a simple configuration | ||
| that consists of a particles in a box in the presence of a | ||
| position-dependent repulsive force that makes the center of the box a less |
There was a problem hiding this comment.
I dont see a file named "free-energy.lmp" in Tutorial 7. Would this be free-energy.lmp? This latter file matches the description shown in the pdf.
| @@ -3822,8 +4157,9 @@ \subsubsection{Method 1: Free sampling} | |||
| adiam 1 3 boxcolor black | |||
| \end{lstlisting} | |||
| A \lmpcmd{dump image} command was also added for system visualization. | |||
There was a problem hiding this comment.
About the dump image command: maybe we can use 500 instead of 50000 for the frequency of the frames? Given that the whole simulation only lasts 50000 timesteps, it would be nice to see the whole progress of the atoms leaving the center region to complement the plot we have in the output.
| and the measured reference density in the reservoir is $\rho_\text{bulk} = 0.0009~\text{\AA{}}^{-3}$.} % SG: check units of rho bulk | ||
| \label{fig:US-density} | ||
| \end{figure} | ||
|
|
||
| \paragraph{LAMMPS input script} | ||
|
|
||
| Open the file named \flecmd{umbrella-sampling.lmp}, which should |
There was a problem hiding this comment.
Btw, the cutoff of the LJ in the input script is set as a variable, so I guess we will need to adapt either here or there for consistency.
| @@ -3981,26 +4323,27 @@ \subsubsection{Method 2: Umbrella sampling} | |||
| \end{lstlisting} | |||
There was a problem hiding this comment.
Same comment previously made about the frequency for the dump image command holds here (500 could be a good value)
|
Okay, I managed to check the other two tutorials. I dont see nothing abnormal about them - there are just some minor comments that were added! |
I am not 100% certain, but I suspect that there is still a bug lurking in the fix shake code that is invoked during minimization. Since you cannot solve the constraint equations, the code implements some stiff harmonic potentials. I've been doing some debugging and the bogus forces and energy do originate with fix shake. Now the big question is: why? |
|
@simongravelle @cecimarques I think I have resolved the issue with using fix shake during minimization. It turned out to be a significant refactor of the code which also means that all reference outputs for simulations using fix shake with minimization need to be recreated. The good news is that the algorithm is now more robust and one can use larger force constants without worrying about bogus results or crashes. Please note that this is an issue of LAMMPS itself and not only LAMMPS-GUI. I will thus also backport the changes to the 22Jul2025 stable version. There are updated LAMMPS-GUI binaries now available at: https://download.lammps.org/testing/ |
solved |
Indeed I found many more: 
They should be all fixed now. |
|
@simongravelle Here is how we search for non-ASCII characters in the LAMMPS source and documentation adapted for the tutorial input files: This still finds a non-ASCII comment: |
Thank you very much. Fixed. Simon |
No description provided.